Set ethernet and wifi MAC address #21

ghost · 2016-12-01T13:24:34Z

This connects #19.
This connects #15

ghost · 2016-12-01T13:27:27Z

The openwrt image must be changed:

set_mac init script must be removed
uccp420wlan init script must be removed
Add 93-uccp420wlan file in /etc/modules.d with the following content:

mac80211
uccp420wlan

Avinash95 · 2016-12-01T13:28:44Z

board/imgtec/pistachio_bub/fdt.c

+	||  check_otp_version(VERSION_REG1_OFFSET))
+		return;
+
+	node = fdt_path_offset(blob, "/uccp@18480000");


should we add an alias for Wi-Fi node as we do for ethernet, as change in dts node name will require a change in the code ?

Avinash95 · 2016-12-01T13:30:29Z

board/imgtec/pistachio_bub/fdt.c

+		return;
+	}
+
+	/* Set Wifi STA and AP MAC address in device tree */


Should we check whether the addresses are valid before writing ?

Avinash95 · 2016-12-01T13:38:05Z

board/imgtec/pistachio_bub/otp.c

+		return -1;
+	}
+
+	if (version != 1) {


can we keep the 1 as macro ?

I am not sure this arch will work from a backwards compatibility point of view. Imagine we had to release a version 2 OTP layout that was a superset of version 1. The code change should be as simple as here is the extra stuff to read if you see version 2. i.e. it should hit the version 1 code already here (not duplicate it) and then carry on to any further versions.

We would never change layouts already released meaning you can always assume a new version is a superset. So based on this how about we drop this file and just wrap the read_otp calls in conditions of greater than version N (in this case 1)?

Avinash95 · 2016-12-01T13:40:03Z

board/imgtec/pistachio_bub/pistachio.c

+
+#ifdef CONFIG_WINBOND_OTP
+	if (!is_valid_ethaddr(mac_addr)
+	&&  !check_otp_version(VERSION_REG0_OFFSET)) {


Move this logical AND to the end of above line

This would conflict with the coding style guidelines of u-boot.

Avinash95 · 2016-12-01T13:45:30Z

board/imgtec/pistachio_bub/pistachio.c

+		else
+			printf("Could not read MAC address from OTP\n");
+	}
+#endif
 #ifdef CONFIG_OF_CONTROL


Enabling this config means, we wont be setting the mac address from OTP, is this what we want ?

Enabling CONFIG_OF_CONTROL means that we will retrieve the MAC address from the DTB if we did not find a valid MAC in the OTP.

Ham22 · 2016-12-01T13:56:48Z

board/imgtec/pistachio_bub/otp.h

+
+#define VERSION_REG0_OFFSET		0x1002
+#define VERSION_REG1_OFFSET		0x2002
+


Seeing as you've added this perhaps we could move all otp layout defines into here?

Ham22 · 2016-12-01T14:17:53Z

board/imgtec/pistachio_bub/fdt.c

+
+	return 0;
+}
+int ft_board_setup(void *blob, bd_t *bd)


Whats the need for the weak ref, no one is going to override us right? Just define ft_board_setup and call fixup_wifi directly from it as we have no cpu setup to do.

removed weak ref

Avinash95 · 2016-12-02T12:14:32Z

board/imgtec/pistachio_bub/fdt.c

+
+DECLARE_GLOBAL_DATA_PTR;
+
+static void fixup_wifi(void *blob)


should we just keep this function to fix wifi mac addresses, and write different one to overwrite calibration data ?

Ham22 · 2016-12-02T12:39:49Z

include/winbond-otp.h

+#ifndef WINBOND_OTP_H
+#define WINBOND_OTP_H
+
+#define MAC_ADDR_LEN 6


Nothing to do with this chip. It too can go to otp.h as it's related to layout?

nikhil-zinjurde-imgtec · 2016-12-02T13:12:59Z

board/imgtec/pistachio_bub/fdt.c

+	||  check_otp_version(VERSION_REG1_OFFSET))
+		return;
+
+	node = fdt_path_offset(blob, "wifi0");


I think this should be "wlan0" to keep in line with the interface names

see my comment on CreatorDev/openwrt#117

abhijit-mahajani-imgtec · 2016-12-02T15:14:10Z

Verified that eth and wifi STA mac addresses are assigned properly.
However WiFi AP mac address looks incorrect.

root@OpenWrt:/# hexdump -n 6 -v -e '/1 "%02X"' /proc/device-tree/uccp@18480000/m
ac-address0
0019F5890D3Broot@OpenWrt:/#
root@OpenWrt:/#
root@OpenWrt:/#
root@OpenWrt:/# hexdump -n 6 -v -e '/1 "%02X"' /proc/device-tree/uccp@18480000/m
ac-address1
68E8FC0F68E8root@OpenWrt:/#

Signed-off-by: Avinash Tahakik <[email protected]> Signed-off-by: Francois Berder <[email protected]>

ethernet MAC address is stored in the OTP, read it and write it into dtb, so kernel can use updated MAC address Signed-off-by: Avinash Tahakik <[email protected]> Signed-off-by: Francois Berder <[email protected]>

The MAC address for Wifi STA and AP are read from the OTP, then written in the wifi driver node of the device-tree. Concerning calibration data, only DCXO is read from the OTP and overwrites the first byte of the calibration parameter in the device tree. Signed-off-by: Francois Berder <[email protected]>

The version of register 0 and 1 must be greater or equal to 1, otherwise do not attempt to read any data from the OTP. Signed-off-by: Francois Berder <[email protected]>

ghost added the s: review label Dec 1, 2016

Avinash95 reviewed Dec 1, 2016

View reviewed changes

Ham22 requested changes Dec 1, 2016

View reviewed changes

Avinash95 self-assigned this Dec 2, 2016

ghost force-pushed the task-15 branch 6 times, most recently from 77f2eb1 to 2a6812e Compare December 2, 2016 11:59

Avinash95 reviewed Dec 2, 2016

View reviewed changes

Ham22 requested changes Dec 2, 2016

View reviewed changes

nikhil-zinjurde-imgtec reviewed Dec 2, 2016

View reviewed changes

ghost force-pushed the task-15 branch from 2a6812e to 9a332d4 Compare December 2, 2016 13:17

Ham22 unassigned Avinash95 Dec 2, 2016

ghost force-pushed the task-15 branch from 9a332d4 to a7225f4 Compare December 2, 2016 15:14

Avinash95 added 2 commits December 2, 2016 15:40

Add driver to read OTP data

3be46f0

Signed-off-by: Avinash Tahakik <[email protected]> Signed-off-by: Francois Berder <[email protected]>

Set ethernet mac address read from OTP

20b07e3

ethernet MAC address is stored in the OTP, read it and write it into dtb, so kernel can use updated MAC address Signed-off-by: Avinash Tahakik <[email protected]> Signed-off-by: Francois Berder <[email protected]>

ghost force-pushed the task-15 branch from a7225f4 to b6142de Compare December 2, 2016 15:41

francois-berder added 2 commits December 2, 2016 15:50

Check version register of the OTP

842f390

The version of register 0 and 1 must be greater or equal to 1, otherwise do not attempt to read any data from the OTP. Signed-off-by: Francois Berder <[email protected]>

ghost force-pushed the task-15 branch from b6142de to 842f390 Compare December 2, 2016 15:51

Ham22 approved these changes Dec 3, 2016

View reviewed changes

Ham22 merged commit 455f049 into CreatorDev:2015.10 Dec 3, 2016

Ham22 removed the s: review label Dec 3, 2016


		#define VERSION_REG0_OFFSET 0x1002
		#define VERSION_REG1_OFFSET 0x2002

Set ethernet and wifi MAC address #21

Set ethernet and wifi MAC address #21

Uh oh!

Conversation

ghost commented Dec 1, 2016 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Dec 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abhijit-mahajani-imgtec commented Dec 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ghost commented Dec 1, 2016 •

edited by ghost

Loading

abhijit-mahajani-imgtec commented Dec 2, 2016 •

edited

Loading